- 
                Notifications
    
You must be signed in to change notification settings  - Fork 13.9k
 
          bootstrap: Use cargo's build.warnings=deny rather than -Dwarnings
          #148332
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This has two major advantages. First, it makes us less dependent on the
rustc shim, which is nice but not super important. More importantly, it
gives us much nicer caching properties.
Previously, `x build --warnings warn && x build --warnings deny` would
rebuild all of bootstrap, and if there were any warnings emitted on the
last build of the compiler, they would *not* fail the build, because
cargo would cache the output rather than rerunning the shim.
After this change, bootstrap rebuilds instantly, and cargo knows that it
should fail the build but *without* invalidating the cache.
Here is an example of rebuilding bootstrap after a switch from
warn->deny:
```
INFO: Downloading and building bootstrap before processing --help command.
      See src/bootstrap/README.md for help with common commands.
Building bootstrap
   Compiling bootstrap v0.0.0 (/Users/jyn/src/ferrocene3/src/bootstrap)
warning: unused variable: `x`
    --> src/bootstrap/src/core/builder/mod.rs:1792:13
     |
1792 |         let x: ();
     |             ^
     |
     = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default
help: if this is intentional, prefix it with an underscore
     |
1792 |         let _x: ();
     |             +
help: you might have meant to pattern match on the similarly named constant `_`
     |
1792 -         let x: ();
1792 +         let utils::render_tests::_: ();
     |
warning: `bootstrap` (lib) generated 1 warning (run `cargo fix --lib -p bootstrap` to apply 1 suggestion)
    Finished `dev` profile [unoptimized] target(s) in 5.14s
error: warnings are denied by `build.warnings` configuration
failed to run: /Users/jyn/src/ferrocene3/build/aarch64-apple-darwin/stage0/bin/cargo build --jobs=default --manifest-path /Users/jyn/src/ferrocene3/src/bootstrap/Cargo.toml -Zroot-dir=/Users/jyn/src/ferrocene3 -Zwarnings
```
building the compiler from scratch with `deny`: https://gist.github.com/jyn514/493ed26c2aa6f61bf47c21e75efa2175
and rebuilding the compiler after switching from deny->warn (note that it reuses the whole cache, there are no invalidations):
```
$ x c compiler
Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.15s
Checking stage1 compiler artifacts{rustc-main, rustc_abi, rustc_arena, rustc_ast, rustc_ast_ir, rustc_ast_lowering, rustc_ast_passes, rustc_ast_pretty, rustc_attr_parsing, rustc_baked_icu_data, rustc_borrowck, rustc_builtin_macros, rustc_codegen_llvm, rustc_codegen_ssa, rustc_const_eval, rustc_data_structures, rustc_driver, rustc_driver_impl, rustc_error_codes, rustc_error_messages, rustc_errors, rustc_expand, rustc_feature, rustc_fluent_macro, rustc_fs_util, rustc_graphviz, rustc_hashes, rustc_hir, rustc_hir_analysis, rustc_hir_id, rustc_hir_pretty, rustc_hir_typeck, rustc_incremental, rustc_index, rustc_index_macros, rustc_infer, rustc_interface, rustc_lexer, rustc_lint, rustc_lint_defs, rustc_llvm, rustc_log, rustc_macros, rustc_metadata, rustc_middle, rustc_mir_build, rustc_mir_dataflow, rustc_mir_transform, rustc_monomorphize, rustc_next_trait_solver, rustc_parse, rustc_parse_format, rustc_passes, rustc_pattern_analysis, rustc_privacy, rustc_proc_macro, rustc_public, rustc_public_bridge, rustc_query_impl, rustc_query_system, rustc_resolve, rustc_sanitizers, rustc_serialize, rustc_session, rustc_span, rustc_symbol_mangling, rustc_target, rustc_thread_pool, rustc_trait_selection, rustc_traits, rustc_transmute, rustc_ty_utils, rustc_type_ir, rustc_type_ir_macros, rustc_windows_rc} (stage0 -> stage1, aarch64-apple-darwin)
warning: function `foo` is never used
  --> compiler/rustc_hashes/src/lib.rs:16:4
   |
16 | fn foo() {}
   |    ^^^
   |
   = note: `#[warn(dead_code)]` (part of `#[warn(unused)]`) on by default
warning: `rustc_hashes` (lib) generated 1 warning
    Finished `release` profile [optimized + debuginfo] target(s) in 0.53s
Build completed successfully in 0:00:08
```
    
          
 Another advantage is you get clear signal what is only a warning (at least for me that is nice)  | 
    
bootstrap: Use cargo's `build.warnings=deny` rather than -Dwarnings
This has two major advantages. First, it makes us less dependent on the rustc shim, which is nice but not super important. More importantly, it gives us much nicer caching properties.
Previously, `x build --warnings warn && x build --warnings deny` would rebuild all of bootstrap, and if there were any warnings emitted on the last build of the compiler, they would *not* fail the build, because cargo would cache the output rather than rerunning the shim.
After this change, bootstrap rebuilds instantly, and cargo knows that it should fail the build but *without* invalidating the cache.
<details><summary>An example of rebuilding bootstrap after a switch from warn->deny:</summary>
```
INFO: Downloading and building bootstrap before processing --help command.
      See src/bootstrap/README.md for help with common commands.
Building bootstrap
   Compiling bootstrap v0.0.0 (/Users/jyn/src/ferrocene3/src/bootstrap)
warning: unused variable: `x`
    --> src/bootstrap/src/core/builder/mod.rs:1792:13
     |
1792 |         let x: ();
     |             ^
     |
     = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default
help: if this is intentional, prefix it with an underscore
     |
1792 |         let _x: ();
     |             +
help: you might have meant to pattern match on the similarly named constant `_`
     |
1792 -         let x: ();
1792 +         let utils::render_tests::_: ();
     |
warning: `bootstrap` (lib) generated 1 warning (run `cargo fix --lib -p bootstrap` to apply 1 suggestion)
    Finished `dev` profile [unoptimized] target(s) in 5.14s
error: warnings are denied by `build.warnings` configuration
failed to run: /Users/jyn/src/ferrocene3/build/aarch64-apple-darwin/stage0/bin/cargo build --jobs=default --manifest-path /Users/jyn/src/ferrocene3/src/bootstrap/Cargo.toml -Zroot-dir=/Users/jyn/src/ferrocene3 -Zwarnings
```
</details>
building the compiler from scratch with `deny`: https://gist.github.com/jyn514/493ed26c2aa6f61bf47c21e75efa2175
<details><summary>and rebuilding the compiler after switching from deny->warn (note that it reuses the whole cache, there are no invalidations):</summary>
```
$ x c compiler
Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.15s
Checking stage1 compiler artifacts{rustc-main, rustc_abi, rustc_arena, rustc_ast, rustc_ast_ir, rustc_ast_lowering, rustc_ast_passes, rustc_ast_pretty, rustc_attr_parsing, rustc_baked_icu_data, rustc_borrowck, rustc_builtin_macros, rustc_codegen_llvm, rustc_codegen_ssa, rustc_const_eval, rustc_data_structures, rustc_driver, rustc_driver_impl, rustc_error_codes, rustc_error_messages, rustc_errors, rustc_expand, rustc_feature, rustc_fluent_macro, rustc_fs_util, rustc_graphviz, rustc_hashes, rustc_hir, rustc_hir_analysis, rustc_hir_id, rustc_hir_pretty, rustc_hir_typeck, rustc_incremental, rustc_index, rustc_index_macros, rustc_infer, rustc_interface, rustc_lexer, rustc_lint, rustc_lint_defs, rustc_llvm, rustc_log, rustc_macros, rustc_metadata, rustc_middle, rustc_mir_build, rustc_mir_dataflow, rustc_mir_transform, rustc_monomorphize, rustc_next_trait_solver, rustc_parse, rustc_parse_format, rustc_passes, rustc_pattern_analysis, rustc_privacy, rustc_proc_macro, rustc_public, rustc_public_bridge, rustc_query_impl, rustc_query_system, rustc_resolve, rustc_sanitizers, rustc_serialize, rustc_session, rustc_span, rustc_symbol_mangling, rustc_target, rustc_thread_pool, rustc_trait_selection, rustc_traits, rustc_transmute, rustc_ty_utils, rustc_type_ir, rustc_type_ir_macros, rustc_windows_rc} (stage0 -> stage1, aarch64-apple-darwin)
warning: function `foo` is never used
  --> compiler/rustc_hashes/src/lib.rs:16:4
   |
16 | fn foo() {}
   |    ^^^
   |
   = note: `#[warn(dead_code)]` (part of `#[warn(unused)]`) on by default
warning: `rustc_hashes` (lib) generated 1 warning
    Finished `release` profile [optimized + debuginfo] target(s) in 0.53s
Build completed successfully in 0:00:08
```
</details>
thanks to `@epage` for the help finding this!
r? bootstrap
    | 
           Possibly failed in rollup in  @bors r-  | 
    
| 
           @bors try jobs=test-various  | 
    
      
        
              This comment has been minimized.
        
        
      
    
  This comment has been minimized.
bootstrap: Use cargo's `build.warnings=deny` rather than -Dwarnings try-job: test-various
| 
           The job  Click to see the possible cause of the failure (guessed by this bot) | 
    
| 
           💔 Test for 9a6eed8 failed: CI. Failed jobs: 
  | 
    
| 
           oh ah um hm. apparently cargo’s version of this applies to hard warnings, not just to lints. that seems not ideal? sometimes hard warnings are there because we don’t want them to break code, and because they’re not lints they can’t be allowed….  | 
    
| 
           frankly I don’t like the idea of hard warnings in the first place, but given that they exist, I would not expect this behavior from cargo.  | 
    
This has two major advantages. First, it makes us less dependent on the rustc shim, which is nice but not super important. More importantly, it gives us much nicer caching properties.
Previously,
x build --warnings warn && x build --warnings denywould rebuild all of bootstrap, and if there were any warnings emitted on the last build of the compiler, they would not fail the build, because cargo would cache the output rather than rerunning the shim.After this change, bootstrap rebuilds instantly, and cargo knows that it should fail the build but without invalidating the cache.
An example of rebuilding bootstrap after a switch from warn->deny:
building the compiler from scratch with
deny: https://gist.github.com/jyn514/493ed26c2aa6f61bf47c21e75efa2175and rebuilding the compiler after switching from deny->warn (note that it reuses the whole cache, there are no invalidations):
thanks to @epage for the help finding this!
r? bootstrap